Daniel van Vugt [Thu, 29 Aug 2024 07:19:14 +0000 (15:19 +0800)]
shell/app: Warn instead of crashing if disposed before startup notification
This is currently easy to reproduce by launching and immediately closing
Chrome, then closing the shell. But automated crash reports show it can
also happen while using the shell, during a garbage collection run.
It's currently the top gnome-shell crasher in Ubuntu, and mostly with
Chromium/Electron apps. But regardless of the offender, we shouldn't
ever trust application behaviour to keep the shell alive.
Simon McVittie [Thu, 18 Sep 2025 09:38:19 +0000 (10:38 +0100)]
Generate a versioned dependency for the GLib 2.86 transition
GNOME Shell will need to be rebuilt after GLib 2.86 is uploaded to
unstable, so that its typelibs declare a dependency on GioUnix, which
they do not when built against older GLib. Generate a versioned
dependency so that this can be done via either a binNMU or a sourceful
upload.
A race condition can occur where a notification banner gets stuck on screen
and never disappears.
This happens when a new notification needs to be shown while an old one
is in the middle of its hiding animation. The process to show the new
notification calls `remove_all_transitions()`, which forcefully stops
the hiding animation of the old one.
The cleanup logic, which destroys the old banner actor, was located in
the `onComplete` handler of the animation. Since the animation was
interrupted and not completed, this handler was never called, leaving a
"zombie" banner on screen.
To fix this, the call to the cleanup function is moved from the
`onComplete` handler to an `onStopped` handler. This handler is
guaranteed to be called whenever an animation stops, regardless of
whether it finished successfully or was interrupted. This ensures the
banner is always cleaned up correctly.
Closes: https://gitlab.gnome.org/GNOME/gnome-shell/-/issues/6006
Part-of: <https://gitlab.gnome.org/GNOME/gnome-shell/-/merge_requests/3861>
(cherry picked from commit 93a79cf61b2b404357e6f637cc9e7f324fb27a69)
Jonas Ådahl [Fri, 5 Sep 2025 13:53:52 +0000 (15:53 +0200)]
popupMenu: Queue relayout on popup menu parent when opening
The position of the menu depends on the position of the "source actor",
but the source actor is not a parent of the menu, but typically a
sibling.
Since we depend on the source actor position for placing the popup menu,
it's important that the source actor gets allocated before we allocate.
Currently the only way to ensure that the source actor is allocated
before the popup menu is by handling the layout via the parent actor, as
the order of the allocations done when queuing shallow relayouts is
currently undefined. As of d94638acae ("clutter/actor: Make child
position changes a noop when possible") in mutter,
Clutter.Actor.set_child_above_sibling() has a fast path when the
position doesn't change, which means the parent was did not get its
relayout queued. This caused the popup menu to end up being allocated
before the source actor, thus using an out of date source actor
allocation to base its position calculation on.
To fix this, queue a relayout on the parent actor unconditionally, after
inserting it into the parent actor, to let us be able to rely on the
source actor having been allocated before the popup menu itself getting
allocated.
Closes: https://gitlab.gnome.org/GNOME/mutter/-/issues/4253
Part-of: <https://gitlab.gnome.org/GNOME/gnome-shell/-/merge_requests/3863>
(cherry picked from commit ca198804598daf595d56600b76b0a0fd088252ee)
Jonas Dreßler [Thu, 24 Jul 2025 16:27:29 +0000 (18:27 +0200)]
st/theme-context: Run dispose on theme nodes on theme context changes
When the theme context changes, we clear the theme cache and make the
widget tree throw away and create new theme nodes. That's because the
old theme nodes can no longer be used, they might be referencing old
CSS data, scale factors, or other things.
St itself should never reuse theme nodes after theme context changes,
but of course theme nodes are exposed to users of St, and they can
always keep a theme node around and call a method on it even after a
theme context change.
To avoid those outdated theme nodes accessing outdated pointers into
the Croco structures and causing crashes, force-run their `dispose()`
function when the theme context changes. This should ensure that all
problematic pointers in the theme nodes are cleared, and users of St
can't crash gnome-shell.
Jonas Dreßler [Thu, 24 Jul 2025 17:47:58 +0000 (19:47 +0200)]
st/theme-node: Free properties array on dispose rather than finalize
We're going to use `g_object_run_dispose()` on StThemeNode with the next
commit in order to manually clear the properties array and a few other
things when the StThemeContext changes.
That's because after theme context changes, the CSS properties will no
longer be valid, and accessing them will cause crashes. Even if St behaves
nicely and throws out all theme nodes on theme context changes, there
might still be users holding references to nodes, so we want to ensure that
the old nodes are harmless after theme context changes.
Jonas Dreßler [Thu, 24 Jul 2025 16:51:25 +0000 (18:51 +0200)]
st/widget: Don't animate theme node updates on theme context changes
With commit 4dfa39457e, I was assuming that emitting `::changed` on the
StThemeContext, triggering StWidgets recursive style-changed machinery,
is enough to get rid of all the theme nodes that are in use, ensuring
they're never accessed again after the `::changed` emission.
That assumption turned out to be wrong: `st_widget_recompute_style()`
animates updates of theme nodes using StThemeNodeTransition, and
StThemeNodeTransition keeps the old theme node around, accessing it at
various later points. This caused various crashes on extension
loading/unloading, because that triggers a theme context change, where
a StThemeNodeTransition would later access an outdated theme node,
pointing to CSS properties that are already freed.
That said, reverting 4dfa39457e doesn't seem like the best idea,
because other calls to `st_theme_context_changed()` also seem to make
the same (incorrect) assumption, eg. `st_theme_context_set_theme()`
seems like a good candidate for a similar crash.
Probably St just shouldn't try to animate on theme context changes: The
old theme nodes are no longer usable after those, but
StThemeNodeTransition expects them to be, so that's just a bad idea.
So change the style-changed machinery a bit to allow disabling transitions
and use that to avoid transitions on theme context changes.
Fixes: 4dfa39457e ("st/theme-context: Also remove theme nodes on stylesheet changes") Closes: https://gitlab.gnome.org/GNOME/gnome-shell/-/issues/7339
Part-of: <https://gitlab.gnome.org/GNOME/gnome-shell/-/merge_requests/3809>
(cherry picked from commit 9ed77ef13e809da12567a5d6725ac2525581f28b)
gdm/authPrompt: Preserve the text when switching entry visibility
The text visibility of an auth prompt entry may change dynamically at
runtime, in particular when unlocking the screen with the blank screen
or with the screen shield visible this is happening:
- The user starts typing the PAM credential (may be secret or not)
- Under the hood the PAM conversation starts and actually asks user
for input
- The entry text is not cleared because we use what the user has typed
so far as the preemptive secret.
- The entry is adjusted to follow the requested PAM text visibility
settings.
-> This may lead to switching the actual text entry widget in use
Now, if the first entry that PAM requires is "ECHO ON", like it may
happen in the case in which a visible value is requested, we end up
"clearing" what the user as typed instead of preserving the value as
pre-filled value.
Causing the user typing something wrong instead.
To prevent this we need to restore both the text, the cursor position
and the selection bounds (in case the use pressed some key [binding])
leading to a position or selection change.
In multi-factor authentication cases we may end up clearing the current
entry, but we are potentially leaving a previously filled inactive entry
as it is.
Avoid this, and clear all the entries we have when clearing the prompt.
gdm/authPrompt: Unset the entry reference on destruction
We use the internal entry as the reference to the text entry that is
currently in use, but during destruction we do not unset it, thus the
garbage collector may requires further cycles to actually release it.
Sebastian Keller [Thu, 21 Aug 2025 17:45:21 +0000 (19:45 +0200)]
st/private: Fix texture leak when creating shadow from actor
cogl_pipeline_set_layer_texture() adds its own ref to the texture in
_cogl_pipeline_set_layer_texture_data() when called via
_st_create_shadow_pipeline(), so the initial reference is leaked when
the pointer is stolen.
Fix this by not stealing the pointer, so autoptr can unref the texture.
Before the code was changed to autoptr in 71accdd97 the texture was also
manually unrefed after calling _st_create_shadow_pipeline().
Fixes: 71accdd97 ("st: Adapt to CoglObject removal") Closes: https://gitlab.gnome.org/GNOME/gnome-shell/-/issues/8509
Part-of: <https://gitlab.gnome.org/GNOME/gnome-shell/-/merge_requests/3854>
(cherry picked from commit 6f9483c434c08e5dbaeec919cddbcb050ab4d2da)
Jonas Dreßler [Fri, 1 Mar 2024 17:39:18 +0000 (18:39 +0100)]
st/focus-manager: Check for NULL instead of stage to exit loop
In st_focus_manager_stage_event() we get the current key-focus actor from
the stage, and then enter a loop where we iterate through parents until we
get to the stage. If the key-focus actor is not a descendent of the stage
for some reason, this loop will never exit and we've locked up.
We can easily prevent the endless loop by checking for NULL as the exit
condition instead of checking for the stage, so do that instead.
tray/na-xembed: Destroy socket window when ending the embedding
Currently when embedding a tray icon is ended we are leaving the X11
socket window around as we don't really destroy it, while we're just
listening for its child destruction.
So, when embedding is ending we need to make sure that:
- If the plug window is still alive, move it back to its old parent
(normally that's the root window, but we query for it, to be sure)
- If the plug window has been already destroyed or reparented (by us or
by something else), we've to just destroy the socket window
Closes: https://gitlab.gnome.org/GNOME/gnome-shell/-/issues/7614
Part-of: <https://gitlab.gnome.org/GNOME/gnome-shell/-/merge_requests/3818>
(cherry picked from commit cd39f7ab4725588d237e7b79763384c0e92f41ff)
tray/na-xembed: Use XShape to remove input on socket window
We don't want this window to take any input as, a part from blocking the
events on the shell representation itself, they may potentially make the
plugged window to do anything with the event is processing (such as
showing tooltips). And we don't want this since we only want to send to
those windows only the synthetic events that we explicitly control.
This was already the case before of commit ab60902058 but a similar
approach was not replicated, leading to input events being fully
processed from tray icons plugs.
It requires adding an explicit build dependency on Xext, but this is
something that mutter already depends on so not really a new dependency
Closes: https://gitlab.gnome.org/GNOME/gnome-shell/-/issues/7613
Part-of: <https://gitlab.gnome.org/GNOME/gnome-shell/-/merge_requests/3818>
(cherry picked from commit 2e97a453290983a3f634153b86a560660896421b)
unlockDialog: Do not reset the auth prompt on every tap
Currently we have a tap event tracker that causes that every time a tap
happens in the lock screen, we reset the auth prompt and this can be
particularly annoying at least in three cases:
1. Just clicking everywhere in the screen may lead the unlock entry
content to be cleared
2. Clicking in the screen while an authentication is in progress,
cancels it
3. This may break a multi-factor authentication method, as a single
click may lead previous steps to be cancelled
So, while resetting the auth prompt is important when we're about to
show it, it's not something we want to do while an authentication has
started.
As per this also do not touch the auth prompt sensitivity unless we're
in an idle phase, or we may end up overriding the auth prompt state,
leading for example to a text entry being editable while we're verifying
the secret
Fixes: 37e55df29865dac13656116efdd7abec8056dea9
Part-of: <https://gitlab.gnome.org/GNOME/gnome-shell/-/merge_requests/3852>
(cherry picked from commit 962d1bf12f55bfadfc41eddd6eefb7597b9edde3)
Takao Fujiwara [Wed, 27 Aug 2025 09:00:30 +0000 (18:00 +0900)]
ibusCandidatePopup: Fix scaling set-cursor-location-relative signal handling
Need to convert the raw coordinates to the scaled ones in gnome-shell.
IBus GtkIMModule can calculate the relative coordinates and gnome-shell
needs to provide the actual scale.
Closes: https://gitlab.gnome.org/GNOME/gnome-shell/-/issues/8424
Part-of: <https://gitlab.gnome.org/GNOME/gnome-shell/-/merge_requests/3729>
(cherry picked from commit 2b2456e87d02b36fa85cdf434011f44143688215)
build: Include GioUnix-2.0 when scanning the libshell headers
libshell exposes in the headers types that are part of GioUnix
(DesktopAppInfo), so we should make it an explicit dependency of the
introspection parser, or we may not be able to parse these types.
Related to: https://gitlab.gnome.org/GNOME/glib/-/issues/3744
Kyrill Zorin [Mon, 11 Aug 2025 22:42:36 +0000 (23:42 +0100)]
extensionSystem: Fix extension enabling/disabling order
Move disabling of unneeded extensions before enabling new ones
in _onEnabledExtensionsChanged(). Previously, conflicting
extensions could simultaneously be active briefly, leading to
unclean states, memory leaks, freezes, or crashes.
The old logic also caused unnecessary disable/enable cycles
for newly requested extensions, wasting resources. Running
the disable step first prevents these issues.
In d6779f34dac6c80e178a308314fe59884a65f383, the switch
to shell_app_activate_action() was made without
also wrapping the action parameter in an av variant.
This patch adjusts network.js to use the correct av
variant type when attempting to connect to an
enterprise AP from the quicksettings panel.
network: Clear connectivity queue after portal login
When full connectivity (or rather, non-portal connectivity) is achieved,
we can clear all active portal helpers.
Normally, the portal helper will signal that it is done, but it is
implemented in a racy manner. So here let's do a comprehensive big
hammer solution to guarantee the portal helper definitely gets
eventually closed even if we miss the race window in the portal helper.
(The race is: the portal helper checks connectivity status either
immediately or within a certain amount of time after a navigation policy
decision. This could theoretically be too soon, and if missed, it won't
be rescheduled.)
portalHelper: Add extra check for policy decision type
This check will always be true currently, because there are only three
possible types of policy decisions, and both of the others are already
handled above. But if new policy decision types are ever added in the
future, then we probably don't want to execute this code.
portalHelper: Reduce the connectivity check rate limit
30 seconds is much too long for GNOME Shell to notice that full
connectivity has been achieved. The portal helper is expected to be
closed once connectivity is achieved.
Note this does NOT mean we unconditionally spam the connectivity check
server every 5 seconds. A connectivity check is still only scheduled
when an actual navigation occurs.
portalHelper: Schedule recheck if it's too soon to recheck
Currently we have a 30 second limit on rechecks. A recheck that would
violate the limit is simply dropped. This causes the portal helper to
never close itself.
Instead, schedule a future recheck for as soon as the timeout expires.
This will put an upper bound on how long the portal helper will
unnecessarily remain open.
(I've also changed the logic to unconditionally emit CANCELLED in
vfunc_close_request(), since this seems more correct. This won't
actually affect anything, because gnome-shell just ignores the
CANCELLED status, but in theory it's nice to end things with an
indication that we are finished. Otherwise, CANCELLED will never be
emitted.)
Florian Müllner [Thu, 22 May 2025 01:27:51 +0000 (03:27 +0200)]
dbusServices/extensions: Use regular constructors
Regular constructors have worked for GObject subclasses for a couple
of years now. While there is no functional difference to _init(),
it's simply nicer to use the same syntax for all classes, so quickly
change that before doing further changes.
Jonas Dreßler [Thu, 24 Jul 2025 23:30:29 +0000 (01:30 +0200)]
smartcardManager: Properly disconnect signal connections from tokens
The `token.disconnectAll()` call here is not going to work: The method is
provided by gjs as convenience for native JS objects (added to an object
using `Signals.addSignalMethods()`). The token here meanwhile is a GObject,
which doesn't have a `disconnectAll()` method.
To make it (hopefully) work as intended, use the convenience functions
that we actually do add to GObjects, that is `connectObject()` and
`disconnectObject()`.
Sebastian Keller [Wed, 30 Jul 2025 13:24:29 +0000 (15:24 +0200)]
slider: Don't preserve paths after filling
Prior to 6fd0aac86 the paths were preserved when filling to allow
stroking the path afterwards. The stroke() then would have cleared the
path. Then when the stroking was dropped in that commit, a second fill()
call, this time without preserving the path was added, but missed from
the end arc of the bar level. This then would leak the path to the
slider handle.
Fix this by always using fill() instead of fillPreserve().
magnifier: Scale the sprite to match the current monitor scaling
Compute the sprite texture scale (unfortunately using a workaround, based on
mutter cursor size preferences, tying to figure out the closest texture integer
scaling), and use this value to compute the scaling that should be applied to
the sprite in order to match the current monitor scaling.
screenshot: Fix taking interactive screenshots via D-Bus
The D-Bus method relies on the `screenshot-taken` and `closed` signals
to differentiate successful operations from canceled/failed ones.
However as the `screenshot-taken` signal is emitted asynchronously,
it may end up being emitted after the `closed` signal even in case
of success. Await the result to ensure correct ordering.
Closes: https://gitlab.gnome.org/GNOME/gnome-shell/-/issues/8499
Part-of: <https://gitlab.gnome.org/GNOME/gnome-shell/-/merge_requests/3803>
(cherry picked from commit 4272916830120c0ff858e9b9de5d242a04932632)
network: Do not consider networks without ID as available
In some rare (and weird) cases network manager may provide us
connections without settings (since connection.get_settings() is
actually nullable, and this may lead to have a NULL connection ID, that
eventually breaks the network menu completely:
(gnome-shell:1579214): GNOME Shell-CRITICAL **: 20:27:14.874: Error:
Argument str2 may not be null
Now, despite that specific connection here has valid settings when
loaded from dbus, it may still happen that NM lib fails at other level in
fetching its settings.
That said, we should not fail if a connection has no settings, and
just skip it.
Florian Müllner [Thu, 26 Jun 2025 10:07:07 +0000 (12:07 +0200)]
st/entry: Forward label-actor to internal ClutterText
The StWidget:label-actor property is a convenient way of setting up an
actor's labelled-by relationship. However on StEntry, it is currently
fairly useless because if the entry is focused, the actual keyboard
focus will be on the internal ClutterText rather than the entry itself,
and orca ignores ancestors in most cases.
Address this by tracking the entry's label-actor property, and replicate
the labelled-by relationship on the internal ClutterText actor.
projects / gnome-shell.git / log